Remove unneeded 'fixed' statements from SocketAddress #61006
Conversation
ghost
added
the
area-networking
GrabYourPitchforks
requested review from
BrennanConroy,
JamesNK,
halter73 and
mgravell
as code owners
|
@jkotas @BrennanConroy So this was an interesting little bit of refactoring, but the upshot is: Don't hide the fact that we fundamentally have to interpret a pointer under the covers, since we need to So this means the only unsafe code is this: return pSockaddr->sa_family switch
{
ADDRESS_FAMILY.AF_INET => new SocketAddressIPv4(*(SOCKADDR_IN*)pSockaddr),
ADDRESS_FAMILY.AF_INET6 => new SocketAddressIPv6(*(SOCKADDR_IN6*)pSockaddr),
_ => null
};The |
| // address is network byte order | ||
| // when CsWin32 gets support for inline arrays, remove 'AsReadOnlySpan' call below. | ||
| // https://github.com/microsoft/CsWin32/issues/1086 | ||
| return new IPAddress(_sockaddr.sin6_addr.u.Byte.AsReadOnlySpan()); // TODO: Does scope id matter? |
There was a problem hiding this comment.
IPAddress does accept a scopeId, any reason not to just pass it in?
There was a problem hiding this comment.
TBH I don't know. The original code didn't pass it in, and I tried to avoid changing the observable behavior in this PR. (The "TODO" comment is copied from the original code.)
3 participants
Part of the "reduce unsafe" effort across the dotnet org.
A prototype static analysis rule identified SocketAddress as being a good candidate to remove a
fixedstatement. We can't entirely remove unsafe from the target method because the value passed in is a pointer, but at least we can remove the unsafe code surrounding the buffer access.